-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Major update #26
base: master
Are you sure you want to change the base?
Major update #26
Conversation
yackermann
commented
Dec 9, 2020
- Fixed issue with Noble integration
- Moved to @abandonware/noble
- Fixed error with crashing when WinRT fails to return name
- Updated outdated libraries
- Other fixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this, the changes look reasonable. Two minor things then it's good from my side
Need to get this merged as its the best version out there so far. Also would be nice to convert the code to work against the latest Windows 10 SDK. |
@tony-gutierrez @geovie I've updated packages, and fixed specified issues. Appologies for the delay. |
+1 for getting this code merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except some places which will need a return statement.
Thanks again.
auto& reader = DataReader::FromBuffer(value); | ||
Data data(reader.UnconsumedBufferLength()); | ||
reader.ReadBytes(data); | ||
mEmit.Read(uuid, serviceId, characteristicId, data, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a return statement here else read
will be emitted twice
auto& reader = DataReader::FromBuffer(value); | ||
Data data(reader.UnconsumedBufferLength()); | ||
reader.ReadBytes(data); | ||
mEmit.ReadValue(uuid, serviceId, characteristicId, descriptorId, data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
auto& reader = DataReader::FromBuffer(value); | ||
Data data(reader.UnconsumedBufferLength()); | ||
reader.ReadBytes(data); | ||
mEmit.ReadHandle(uuid, handle, data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
Does look like some returns should be added. |